-
Notifications
You must be signed in to change notification settings - Fork 48
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I'm not sure we need to update or include the dates anymore. |
@mhdawson because of the merge? |
@WaleedAshraf nodejs/TSC#195 (comment)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the conversation in nodejs/TSC#195 (comment) I don't think we should be changing the license header. For new license we can avoid the date... but it doesn't have legal reasons to change it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ogaston Anyways, thanks for the first contribution try. 👍keep going.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution @ogaston! 👍🏽
Because of the legal requirements outlined in the comments above, we'll refrain from changing the license date. Future licenses will not be including a date anymore and old licenses with a date will be allowed to remain that way.
I'll be closing this pull request without merging. Please do continue to build on your new familiarity with the Node.js project by continuing to contribute in issues and PRs! 🧡
Glad to have you onboard!
No description provided.